test(submit-cves): fix main CI — align missing-URL tests with error-status contract - #99
Merged
Merged
Conversation
Two competing PRs (#92, #93) merged with conflicting expectations for submit_cves() when no webhook URL is configured. PR #93 deliberately moved URL validation inside the try block so a missing URL returns {"status": "error"} instead of raising ValueError (see source comment), and its tests in test_bug_fixes.py assert that contract. PR #92 added TestSubmitCvesMissingUrl in test_submit_cves_and_logger.py that still expects the removed `raise ValueError`, breaking main CI (2 failed). Update the two stale tests to assert the error-status dict contract, matching the source's documented behavior and the passing sibling tests. Drop the now-unused pytest import (F401). Full suite: 1154 passed, ruff clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
mainCI is red (Tests (Python 3.11)andTests (Python 3.12)) after PR #92 and #93 merged. Two tests fail withDID NOT RAISE ValueError:tests/test_submit_cves_and_logger.py::TestSubmitCvesMissingUrl::test_no_url_raises_value_errortests/test_submit_cves_and_logger.py::TestSubmitCvesMissingUrl::test_no_url_error_message_mentions_config_tomlRoot cause — a merge collision between two competing PRs
tryblock insubmit_cves()so a missing URL returns{"status": "error"}instead of raisingValueErrorto the caller. There's an explicit source comment documenting this, and its tests intest_bug_fixes.py::TestSubmitCvesMissingUrlassert the error-status contract (they pass).TestSubmitCvesMissingUrlclass intest_submit_cves_and_logger.pythat still expects the old behavior (pytest.raises(ValueError)).Both merged independently, so the source now follows #93's design while #92's stale tests still assert the removed raise → CI breaks.
Fix
Update the two stale tests to assert the current, intended contract (error-status dict +
config.tomlhint in the message), matching the source and the passing sibling tests intest_bug_fixes.py. Also drops the now-unusedpytestimport (ruff F401).This is the correct direction: reverting the source would break #93's deliberate design and its passing test suite.
Verification
ruff check+ruff format --checkclean